Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

4.22.01 fhd patch merged upstream (4.24) + bump all 4.19 boards to 4.22.01 #1604

Merged
merged 7 commits into from
Mar 26, 2024

Conversation

tlaurion
Copy link
Collaborator

@tlaurion tlaurion commented Feb 5, 2024

Revisiting 4.22 coreboot bump with fhd patch (superseeds and replaces #1524), Might fix #1603)

- patches/coreboot-4.22.01/0001-x230-fhd-variant.patch created per
  - git fetch https://review.coreboot.org/coreboot refs/changes/50/28950/27 && git format-patch -1 --stdout FETCH_HEAD > ~/heads/patches/coreboot-4.22.01/0001-x230-fhd-variant.patch
- all boards configs bumped with:
  - sed 's/4.19/4.22.01/g' boards/*/*.config -i
  - grep -Rn 4.22 boards/ | awk -F "/" {'print $2'}| while read line; do make BOARD=$line coreboot.save_in_oldconfig_format_in_place ; done
  - sed 's/4.19/4.22.01/g' .circleci/config.yml -i
- CircleCI caches updated
- modules/coreboot: remove 4.19 references
- All 4.19 boards switched to TCG event log format which segfaults `cbmem -L` https://github.com/linuxboot/heads/issues/1608#issuecomment-1953036042
  - Note that Purism boards already switched to TCG format where https://github.com/linuxboot/heads/pull/1609 doesn't fix anything (coreboot custom event log format is broken since 4.21
    - **More details under https://github.com/linuxboot/heads/issues/1608 which isn't blocking this PR** 

Testing report at #1604 (comment)


4.22 didn't include the patchset in time as #1523 related.
Hopefully this is the last time we pick a patchset. Please board owners, report your edp/fhd kit (PRECISELY) and passing/failing results under https://review.coreboot.org/c/coreboot/+/28950 EDIT: https://review.coreboot.org/coreboot refs/changes/50/28950/27 was merged.

@tlaurion
Copy link
Collaborator Author

tlaurion commented Feb 5, 2024

@JonathonHall-Purism
Copy link
Collaborator

(realize it's WIP but took a quick look 🙂 ) Do any boards remain on 4.19 after this change? If not, IMO we should remove it from modules/coreboot.

@pcm720
Copy link

pcm720 commented Feb 5, 2024

Works perfectly on eDP 4.0-modded X230 with 1440p display.
"Unknown TPM log specification" doesn't prevent TPM from unlocking encrypted root partition.

@tlaurion tlaurion marked this pull request as draft February 5, 2024 18:03
@tlaurion
Copy link
Collaborator Author

tlaurion commented Feb 5, 2024

Works perfectly on eDP 4.0-modded X230 with 1440p display. "Unknown TPM log specification" doesn't prevent TPM from unlocking encrypted root partition.

Cannot replicate with qemu-coreboot-whpitail-tpm1/tpm2 boards, this is weird.

@tlaurion
Copy link
Collaborator Author

tlaurion commented Feb 5, 2024

Passing to tpm 1.2 event log format makes cbmem crash... Something has not followed upstream changes inside of cbmem?!?!

Another rabbit hole.

@JonathonHall-Purism
Copy link
Collaborator

JonathonHall-Purism commented Feb 5, 2024

@tlaurion I'm working on PureBoot 29 release candidates now (updating coreboot to 4.22.01 there as well) and I get something similar with cbmem -L, but I'm not sure it's exactly the same.

It does appear to show the TPM log correctly, but then it shows a huge garbage entry and crashes. Sealing still works because the correct part of the log is interpreted correctly by tpmr. It looks like cbmem is wandering off into uninitialized space following the TPM log (but haven't confirmed that).

Is that similar to what you see?

Next steps IMO would be to build cbmem in the host OS see if it's the same (see if it's specific to the Heads build/environment or not, since it would surprise me if it was doing this across the board upstream), then probably bisect coreboot and/or cbmem based on that result.

@tlaurion
Copy link
Collaborator Author

tlaurion commented Feb 5, 2024

Checking at history of cbmem https://github.com/coreboot/coreboot/commits/7201602a18b63fc5236f025d22dc726637866cb6/util/cbmem/cbmem.c, it seems something went wrong either at coreboot/coreboot@6da6268 when supporting tpm1.2/tpm2.0 format or coreboot/coreboot@fc2f304 when attempting to standardize c99 arrays.

@tlaurion I'm working on PureBoot 29 release candidates now (updating coreboot to 4.22.01 there as well) and I get something similar with cbmem -L, but I'm not sure it's exactly the same.

On x230 I only get "Unknown TPM log specification:" (note the ':' and nothing else) at
https://github.com/coreboot/coreboot/blob/fc2f304f062bc51cf51ad999caf13a6acc64a1b3/util/cbmem/cbmem.c#L1059-L1061

It does appear to show the TPM log correctly, but then it shows a huge garbage entry and crashes. Sealing still works because the correct part of the log is interpreted correctly by tpmr. It looks like cbmem is wandering off into uninitialized space following the TPM log (but haven't confirmed that).

Is that similar to what you see?

No. I get this when I switch the TPM log format to 1.2 though, not when using coreboot custom format that we currently use.

Next steps IMO would be to build cbmem in the host OS see if it's the same (see if it's specific to the Heads build/environment or not, since it would surprise me if it was doing this across the board upstream), then probably bisect coreboot and/or cbmem based on that result.

Well, TBH I'm not sure a lot of other use so much measured boot outside of Heads @JonathonHall-Purism ?
This shows its not tested a lot, or as you said, since cbmem is built by musl-cross-make, that might as well be an undesired artifact of those?

@tlaurion
Copy link
Collaborator Author

tlaurion commented Feb 5, 2024

@JonathonHall-Purism tested bumping musl-cross-make to latest avail commit but same error on x230-maximized and clean built: "Unknown TPM log specification:"

https://github.com/tlaurion/heads/tree/422_fhd_newer-musl

@tlaurion
Copy link
Collaborator Author

tlaurion commented Feb 13, 2024

@JonathonHall-Purism

Well, TBH I'm not sure a lot of other use so much measured boot outside of Heads @JonathonHall-Purism ?
This shows its not tested a lot, or as you said, since cbmem is built by musl-cross-make related doubts, which is compiler buillding everything tools.cpio related under Heads, including cbmem here), that might as well be an undesired artifact of those? (modified quote)

I took the time to use rom produced here (4.22.01 tag release codebase built with measured boot and coreboot custom event log format) and test cbmem built on top of ubuntu 23.10/debian trixie OS (to remove musl-cross-make doubts).

Interestingly enough:

  • sudo ~/Downloada/coreboot-4.22.01/util/cbmem/cbmem -L : Unknown TPM log specification: : FAIL
  • sudo ~/Downloada/coreboot-4.19/util/cbmem/cbmem -L : coreboot TPM log: [...] : OK

So now, let's test cbmem behavior in between those release versions:

  • sudo ~/Downloada/coreboot-4.21/util/cbmem/cbmem -L : Unknown TPM log specification: : FAIL
  • sudo ~/Downloada/coreboot-4.20.1/util/cbmem/cbmem -L : coreboot TPM log: [...] : OK

So cbmem -L broke somewhere between 4.20.1 and 4.21.


Raised awareness under #coreboot channel at https://matrix.to/#/!EhaGFZyYcbyhdSgStq:matrix.org/$DJW_mF5d1PfhJIvoM-_HvX8s3BpzNvtNu77auhvKBo0?via=matrix.org&via=sibnsk.net&via=datanauten.de. @miczyg1 said he would take a look at https://matrix.to/#/!EhaGFZyYcbyhdSgStq:matrix.org/$34SHe96Ef5Qrv2pIXtmi2lqpSwnh1GsKXlZCR9I7PpY?via=matrix.org&via=sibnsk.net&via=datanauten.de

@tlaurion
Copy link
Collaborator Author

tlaurion commented Feb 14, 2024

Thanks to you all, edp/fhd was merged upstream (~4.24 to be)

EDIT: coreboot switches to date based releases. So coreboot 24.02 it will be...

@tlaurion
Copy link
Collaborator Author

Testing FHD+4.23 coreboot is under https://github.com/tlaurion/heads/tree/pre423-edp_fhd_in-tpm_tcg_event_log-CircleCi_coreboot_cache_fixed

As of now, the branch is just to show that cbmem -L fails (just as here) but for fixes to be made upstream as reported at #1609 (comment)

@tlaurion
Copy link
Collaborator Author

tlaurion commented Feb 19, 2024

Updated to match coreboot merged https://review.coreboot.org/c/coreboot/+/28950. Might need manual fix (since other things changed for 4.23)

EDIT: yep. Manual fixes needed for 4.22

@tlaurion
Copy link
Collaborator Author

tlaurion commented Feb 19, 2024

Traces to debug coreboot custom TPM event log for 4bb8317 at #1608 (comment)

Switching to TPM TCG event log format on next commit

@tlaurion
Copy link
Collaborator Author

tlaurion commented Feb 19, 2024

Logs for 6b61f61 segfaulting TCG event log cbmem -L output under #1608 (comment)

Not a blocker: tpmr (pcr replay) works, but dmesg will show segfault until fixed.

@tlaurion tlaurion changed the title WiP 4.22.01 fhd patch test + bump all 4.19 boards to 4.22.01 4.22.01 fhd patch test + bump all 4.19 boards to 4.22.01 Feb 19, 2024
@tlaurion
Copy link
Collaborator Author

@pcm720 can you confirm upstream merged+modified(minimal: only filename changes) edp/fhd patchset works as expected here? Should.

@tlaurion
Copy link
Collaborator Author

tlaurion commented Feb 19, 2024

@lethedata: #1603 (comment) "reboot issue" might not be fixed by this and as of today, is outside of my comprehension.

@tlaurion tlaurion changed the title 4.22.01 fhd patch test + bump all 4.19 boards to 4.22.01 4.22.01 fhd patch merged upstream (4.24) + bump all 4.19 boards to 4.22.01 Feb 19, 2024
@tlaurion tlaurion marked this pull request as ready for review February 19, 2024 19:49
@tlaurion
Copy link
Collaborator Author

tlaurion commented Feb 19, 2024

Tested:

Needs testing:

As of now, HOTP+TPM DUK+suspend+resume works as expected and no regression reported on tested boards above.

@pcm720
Copy link

pcm720 commented Feb 19, 2024

@pcm720 can you confirm upstream merged+modified(minimal: only filename changes) edp/fhd patchset works as expected here? Should.

Yes, everything works as expected, no issues with disk decryption or OS reboots.

@tlaurion
Copy link
Collaborator Author

Testing needed, everyone!

@tlaurion
Copy link
Collaborator Author

Untested boards will move to UNTESTED.
UNTESTED boards that were untested prior of this PR will go to UNMAINTAINED in future commit prior of merging.

@natterangell
Copy link
Contributor

natterangell commented Feb 22, 2024 via email

@d-wid
Copy link
Contributor

d-wid commented Feb 23, 2024 via email

@JonathonHall-Purism
Copy link
Collaborator

I'll get to it next Wednesday or, if you don't mind

tlaurion is away until March 20th, so no rush 👍

@tlaurion
Copy link
Collaborator Author

I'll get to it next Wednesday or, if you don't mind

tlaurion is away until March 20th, so no rush 👍

@d-wid @JonathonHall-Purism it was intentional to push for testing before leaving on my side. Last time I asked for testing, it took more then 3weeks to get the results.

So @d-wid and others: please have your testing reports before march 20th. Happy testing!

@srgrint
Copy link
Contributor

srgrint commented Feb 24, 2024

Have tested heads-x220-maximized-v0.2.0-2027-g6b61f61.rom Seems to work fine.

@gaspar-ilom
Copy link
Contributor

I have regression tested W541 (heads-w541-hotp-maximized-v0.2.0-2027-g6b61f61.rom). No findings. You may mark it as tested.

@srgrint
Copy link
Contributor

srgrint commented Feb 26, 2024

Have also tested t440p - also seems to work fine.

@srgrint
Copy link
Contributor

srgrint commented Feb 29, 2024

Tested on my intel graphics T430. Seems to work fine.

@d-wid
Copy link
Contributor

d-wid commented Mar 1, 2024 via email

@tlaurion
Copy link
Collaborator Author

tlaurion commented Mar 22, 2024

Thanks to all testing reports per #1604 (comment), last call for testing for :

Some boards will go to UNTESTED, and current UNTESTED boards will go UNMAINTAINTED.

@natterangell
Copy link
Contributor

natterangell commented Mar 23, 2024 via email

- patches/coreboot-4.22.01/0001-x230-fhd-variant.patch created per
  - git fetch https://review.coreboot.org/coreboot refs/changes/50/28950/23 && git format-patch -1 --stdout FETCH_HEAD > ~/heads/patches/coreboot-4.22.01/0001-x230-fhd-variant.patch
- all boards configs bumped with:
  - grep -Rn 4.22 boards/ | awk -F "/" {'print $2'}| while read line; do make BOARD=$line coreboot.save_in_oldconfig_format_in_place ; done

Signed-off-by: Thierry Laurion <[email protected]>
…eam merged state

git fetch https://review.coreboot.org/coreboot refs/changes/50/28950/27 && git format-patch -1 --stdout FETCH_HEAD > ~/heads/patches/coreboot-4.22.01/0001-x230-fhd-variant.patch

Signed-off-by: Thierry Laurion <[email protected]>
… Makefile.inc (Makefile.mk doesn't exist under 4.22)

Signed-off-by: Thierry Laurion <[email protected]>
…ve them from CircleCI, add Makefile helper and document untested_boards/README.md

Signed-off-by: Thierry Laurion <[email protected]>
@tlaurion
Copy link
Collaborator Author

tlaurion commented Mar 25, 2024

Rebased on master and moved boards from boards/UNTESTED_* to unmaintained_boards/UNMAINTAINED_* per added Makefile helper, removed them from CircleCI.

If there is interest for those boards, they will need to be brought back with proper testing and board owner willingness to maintain/at least test when needed.

See 9fcd5f8 related changes.

@JonathonHall-Purism : merge?

@JonathonHall-Purism
Copy link
Collaborator

Thanks @tlaurion , yes looks good to me for merge 👍

…o more boards can be built reusing cache and where nv41 will be rebuilt if coreboot level cache was not saved)

Signed-off-by: Thierry Laurion <[email protected]>
@tlaurion tlaurion merged commit 05289c0 into linuxboot:master Mar 26, 2024
36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

OS Reboot with i915 Enabled Disables Internal Display - x230 fhd_edp (a.gain)
7 participants